feat: 支持自定义组件props类型提示支持#108
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds runtime prop-type utilities and richer component self typings; emits runtime JSON-declared component objects with stable import identifiers; integrates JSON components into script/template codegen and plugin plumbing; and adds example MPX components plus minor VSCode config updates. ChangesRuntime Prop Types and JSON usingComponents Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/language-core/src/codegen/script/index.ts (1)
35-35: ⚡ Quick winAdd JSDoc for the new required field.
The
scriptSetupImportComponentNamesfield is added without documentation. Consider adding a JSDoc comment explaining what component names are included (e.g., "Set of component names imported via default import from .mpx files in script setup").📝 Proposed documentation
scriptSetupRanges: ScriptSetupRanges | undefined + /** + * Set of component names imported via default import from .mpx files in <script setup>. + * Used for resolving component types in template code generation. + */ scriptSetupImportComponentNames: Set<string> templateCodegen: (TemplateCodegenContext & { codes: Code[] }) | undefined🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/language-core/src/codegen/script/index.ts` at line 35, Add a JSDoc comment above the scriptSetupImportComponentNames field describing its purpose and contents: state that it is a Set<string> containing component names that are imported via default import from .mpx files inside a script setup block (i.e., components brought into scope by `import X from "Y.mpx"` in script setup), note that it is required and used to track which components should be treated as locally registered in generated code, and include a short example usage to clarify semantics.packages/language-core/src/codegen/script/jsonUsingComponents.ts (1)
112-122: ⚡ Quick winConsider adding index suffix for consistency.
The function returns just
${baseName}without any index suffix (line 121). When multiple paths resolve to the same component name, they'll have different indices, but the first one (index 0) won't have a distinguishing suffix. Consider returning${baseName}_${index}or${baseName}${index === 0 ? '' :_${index}}for consistency and to avoid potential name collisions if the same component name appears multiple times.🔧 Proposed refinement for consistency
export function getUsingComponentImportName( componentName: string, index: number, ) { const camelizedName = capitalize(camelize(componentName)) const baseName = camelizedName && identifierRegex.test(camelizedName) ? camelizedName : `component_${index}` - return `${baseName}` + return index > 0 && camelizedName && identifierRegex.test(camelizedName) + ? `${baseName}_${index}` + : baseName }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/language-core/src/codegen/script/jsonUsingComponents.ts` around lines 112 - 122, getUsingComponentImportName currently returns baseName without an index suffix which can collide when the same componentName appears multiple times; update the function (referencing getUsingComponentImportName, camelizedName, identifierRegex, baseName) to append a deterministic index suffix such as `${baseName}_${index}` (or `${baseName}${index === 0 ? '' : `_${index}`}` if you want to keep the original for index 0) so each generated import name is unique and consistent across duplicates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@inspect-extension/components/reference-type-props/component-options.mpx`:
- Around line 17-21: The JSON script block uses the incorrect symbol
"module.export"; update it to the standard Node.js export symbol
"module.exports" in the JSON script (the object assigned to module.exports) so
the export matches the rest of the codebase and other JSON blocks—locate the
script named "json" in component-options.mpx and replace module.export with
module.exports.
In `@inspect-extension/components/reference-type-props/index.mpx`:
- Line 3: This line intentionally uses a numeric literal for the msg prop of the
component-js-setup component to trigger type-checking; add an inline comment
next to the component invocation (component-js-setup msg="{{ 123 }}") stating
that this is test code in reference-type-props and that the number is an
intentional type mismatch for verification of the string-typed msg prop defined
in component-js-setup.mpx.
- Line 2: The parentText prop on the component-options component is receiving a
number ({{ 123 }}) while component-options.mpx defines parentText as type
String; either change the value to a string literal to match the declared type
(e.g., provide a string expression) or add an inline comment near the component
usage (or above the file) stating that the numeric value is intentionally used
to test type-checking; reference the component-options component and its
parentText prop declared in component-options.mpx when making the change so the
mismatch is resolved or explicitly documented.
---
Nitpick comments:
In `@packages/language-core/src/codegen/script/index.ts`:
- Line 35: Add a JSDoc comment above the scriptSetupImportComponentNames field
describing its purpose and contents: state that it is a Set<string> containing
component names that are imported via default import from .mpx files inside a
script setup block (i.e., components brought into scope by `import X from
"Y.mpx"` in script setup), note that it is required and used to track which
components should be treated as locally registered in generated code, and
include a short example usage to clarify semantics.
In `@packages/language-core/src/codegen/script/jsonUsingComponents.ts`:
- Around line 112-122: getUsingComponentImportName currently returns baseName
without an index suffix which can collide when the same componentName appears
multiple times; update the function (referencing getUsingComponentImportName,
camelizedName, identifierRegex, baseName) to append a deterministic index suffix
such as `${baseName}_${index}` (or `${baseName}${index === 0 ? '' :
`_${index}`}` if you want to keep the original for index 0) so each generated
import name is unique and consistent across duplicates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 121ec58c-f129-469c-bdde-f34c59d885f7
📒 Files selected for processing (14)
inspect-extension/components/reference-type-props/component-js-setup.mpxinspect-extension/components/reference-type-props/component-options.mpxinspect-extension/components/reference-type-props/index.mpxinspect-extension/components/script-json/components/list.mpxpackages/language-core/src/codegen/globalTypes/defineComponentTypes.tspackages/language-core/src/codegen/globalTypes/index.tspackages/language-core/src/codegen/script/componentSelf.tspackages/language-core/src/codegen/script/index.tspackages/language-core/src/codegen/script/jsonUsingComponents.tspackages/language-core/src/codegen/script/template.tspackages/language-core/src/codegen/template/element.tspackages/language-core/src/codegen/template/elementProps.tspackages/language-core/src/codegen/template/templateChild.tspackages/language-core/src/plugins/mpx-tsx.ts
💤 Files with no reviewable changes (1)
- packages/language-core/src/codegen/template/templateChild.ts
| @@ -0,0 +1,21 @@ | |||
| <template> | |||
| <component-options parentText="{{ 123 }}"></component-options> | |||
| <component-js-setup msg="{{ 123 }}"></component-js-setup> | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.mpx" | grep -E "(component-js-setup|reference-type-props)" | head -20Repository: mpx-ecology/language-tools
Length of output: 281
🏁 Script executed:
cat -n ./inspect-extension/components/reference-type-props/index.mpxRepository: mpx-ecology/language-tools
Length of output: 668
🏁 Script executed:
cat -n ./inspect-extension/components/reference-type-props/component-js-setup.mpxRepository: mpx-ecology/language-tools
Length of output: 220
🏁 Script executed:
ls -la ./inspect-extension/components/reference-type-props/Repository: mpx-ecology/language-tools
Length of output: 384
Add a comment explaining this is test code for type-checking verification.
The msg prop is defined as string in component-js-setup.mpx, but receives {{ 123 }} (a number). Given this is in the reference-type-props test directory, this type mismatch appears intentional. Add a comment to clarify:
- <component-js-setup msg="{{ 123 }}"></component-js-setup>
+ <!-- Test: intentional type error - number passed to string prop -->
+ <component-js-setup msg="{{ 123 }}"></component-js-setup>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <component-js-setup msg="{{ 123 }}"></component-js-setup> | |
| <!-- Test: intentional type error - number passed to string prop --> | |
| <component-js-setup msg="{{ 123 }}"></component-js-setup> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@inspect-extension/components/reference-type-props/index.mpx` at line 3, This
line intentionally uses a numeric literal for the msg prop of the
component-js-setup component to trigger type-checking; add an inline comment
next to the component invocation (component-js-setup msg="{{ 123 }}") stating
that this is test code in reference-type-props and that the number is an
intentional type mismatch for verification of the string-typed msg prop defined
in component-js-setup.mpx.
wangshunnn
changed the title
2 participants
closes #86
Summary by CodeRabbit
Improvements
New Features
Chores